Integrate nostr-sdk relay options, stream events, and timestamp tweaking#283
Integrate nostr-sdk relay options, stream events, and timestamp tweaking#283kwsantiago merged 7 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughRefactors relay management to use the client's relay pool and RelayOptions, migrates agent event consumption to pool-based streams, adds a TIMESTAMP_TWEAK_RANGE constant and applies tweaked timestamps to many emitted events, and replaces chrono timestamps with nostr_sdk::Timestamp across multiple modules. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
keep-nip46/src/server.rs (1)
515-516: Consider using a shared constant for timestamp tweak range.This uses the literal
0..5whilekeep-frost-netdefinesTIMESTAMP_TWEAK_RANGEfor the same purpose. For consistency and maintainability, consider extracting this to a shared constant (e.g., inkeep-core) or importing it if cross-crate visibility is feasible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@keep-nip46/src/server.rs` around lines 515 - 516, Replace the hardcoded range literal 0..5 passed to Timestamp::tweaked with a shared constant (e.g., TIMESTAMP_TWEAK_RANGE) to match other crates; update the call in the EventBuilder creation (EventBuilder::new(...).custom_created_at(Timestamp::tweaked(...))) to use that constant, and either import TIMESTAMP_TWEAK_RANGE from the existing keep-frost-net/keep-core crate or introduce a public constant in keep-core and use that import so the same identifier is reused across crates for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@keep-nip46/src/server.rs`:
- Around line 515-516: Replace the hardcoded range literal 0..5 passed to
Timestamp::tweaked with a shared constant (e.g., TIMESTAMP_TWEAK_RANGE) to match
other crates; update the call in the EventBuilder creation
(EventBuilder::new(...).custom_created_at(Timestamp::tweaked(...))) to use that
constant, and either import TIMESTAMP_TWEAK_RANGE from the existing
keep-frost-net/keep-core crate or introduce a public constant in keep-core and
use that import so the same identifier is reused across crates for consistency.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
keep-agent/src/client.rskeep-frost-net/src/event.rskeep-frost-net/src/node/descriptor.rskeep-frost-net/src/node/mod.rskeep-nip46/src/server.rs
There was a problem hiding this comment.
🧹 Nitpick comments (2)
keep-agent/src/client.rs (1)
564-612: LGTM!The helper functions
wait_for_relay_connectionandwait_for_any_relay_connectionare well-structured:
- Clear timeout handling with informative error messages
- 100ms polling interval is reasonable
default_relay_opts()provides sensible defaults (reconnect, ping, 10s retry, max 3s latency)Note:
default_relay_opts()is duplicated inkeep-frost-net/src/node/mod.rs. Consider extracting to a shared location inkeep-coreif this configuration should be consistent across components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@keep-agent/src/client.rs` around lines 564 - 612, The function default_relay_opts() is duplicated in another crate; extract this helper into a shared module in keep-core (e.g., a new relay/config helper) and export it so both keep-agent (where default_relay_opts is defined) and keep-frost-net (where it’s duplicated) can import the single implementation; update callers in wait_for_relay_connection and wait_for_any_relay_connection to call the shared default_relay_opts (remove the local copy) and ensure the shared function preserves the same settings (reconnect, ping, retry_interval 10s, adjust_retry_interval, ban_relay_on_mismatch, max_avg_latency 3s) and appropriate visibility and crate features to avoid breaking consumers.keep-frost-net/src/node/mod.rs (1)
1177-1185: Consider extractingdefault_relay_opts()to a shared location.This function is duplicated identically in
keep-agent/src/client.rs(lines 604-612). While the duplication is minor, having a single source of truth for relay configuration would make it easier to maintain consistent behavior across components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@keep-frost-net/src/node/mod.rs` around lines 1177 - 1185, default_relay_opts() is duplicated; extract it to a single, shared public function (e.g., pub fn default_relay_opts() in a new or existing common module like relay/config or utils) so both modules call the same implementation; ensure the function is exported (pub) and that the RelayOptions type and Duration are in scope where it’s used, replace the two local copies with calls to the shared function, and remove the duplicate definitions so there’s one source of truth for relay configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@keep-agent/src/client.rs`:
- Around line 564-612: The function default_relay_opts() is duplicated in
another crate; extract this helper into a shared module in keep-core (e.g., a
new relay/config helper) and export it so both keep-agent (where
default_relay_opts is defined) and keep-frost-net (where it’s duplicated) can
import the single implementation; update callers in wait_for_relay_connection
and wait_for_any_relay_connection to call the shared default_relay_opts (remove
the local copy) and ensure the shared function preserves the same settings
(reconnect, ping, retry_interval 10s, adjust_retry_interval,
ban_relay_on_mismatch, max_avg_latency 3s) and appropriate visibility and crate
features to avoid breaking consumers.
In `@keep-frost-net/src/node/mod.rs`:
- Around line 1177-1185: default_relay_opts() is duplicated; extract it to a
single, shared public function (e.g., pub fn default_relay_opts() in a new or
existing common module like relay/config or utils) so both modules call the same
implementation; ensure the function is exported (pub) and that the RelayOptions
type and Duration are in scope where it’s used, replace the two local copies
with calls to the shared function, and remove the duplicate definitions so
there’s one source of truth for relay configuration.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
keep-agent/src/client.rskeep-core/src/relay.rskeep-frost-net/Cargo.tomlkeep-frost-net/src/event.rskeep-frost-net/src/node/descriptor.rskeep-frost-net/src/node/mod.rskeep-frost-net/src/protocol.rskeep-nip46/src/server.rs
💤 Files with no reviewable changes (1)
- keep-frost-net/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- keep-nip46/src/server.rs
Summary by CodeRabbit
Improvements
Chores